Skip to content

fix!: preserve detail from upstream address parse errors - #1138

Open
Arowolokehinde wants to merge 1 commit into
bitcoindevkit:masterfrom
Arowolokehinde:fix/preserve-address-parse-error-detail
Open

Arowolokehinde wants to merge 1 commit into
bitcoindevkit:masterfrom
Arowolokehinde:fix/preserve-address-parse-error-detail

Conversation

@Arowolokehinde

@Arowolokehinde Arowolokehinde commented Oct 7, 2026 •

Copy link
Copy Markdown

Description

Fixes #1095

AddressParseError discarded the upstream error in seven of ten arms, reporting only a generic label such as "base58 address encoding error". Every variant now carries an error_message holding the upstream detail:

Notes to the reviewers

Why the conversion follows each error's source() instead of relying on to_string() alone.
The detail sits two levels below ParseError::Base58. The wrapping errors use the write_err! macro, which leaves the source out of Display when the std feature is enabled — so to_string() returns "base58 error" and nothing else, and the rest is reachable only through source().

Scope. The issue names Base58, Bech32 and network validation. This also covers the four other arms that discard the error the same way, plus OtherAddressParseErr, which had no payload at all. The change breaks the generated bindings either way. Happy to narrow it to the three named variants if you'd prefer.

Documentation

Changelog

changelog: breaking

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing
  • I've added exactly one changelog:* label
  • I've linked the relevant upstream docs or specs above

Bugfixes:

  • This pull request breaks the existing API
  • I've added tests to reproduce the issue which are now passing
  • I'm linking the issue being fixed by this PR

@Arowolokehinde
Arowolokehinde force-pushed the fix/preserve-address-parse-error-detail branch 2 times, most recently from 7af33d1 to d6acb92 Compare October 7, 2026 11:34
@Arowolokehinde
Arowolokehinde force-pushed the fix/preserve-address-parse-error-detail branch from d6acb92 to 8636ba7 Compare October 7, 2026 11:35
@j-kon

j-kon commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

The error propagation itself looks correct, and I verified the generated Swift/Kotlin API impact.

One question before ACKing: since these unit variants become data-carrying variants, this is a real generated-binding API break. With master currently on 3.2.0-alpha.0 and #1036 tracking breaking changes for 4.0, is this intended to land in 3.2 or should it be tracked for 4.0?

@Arowolokehinde

Copy link
Copy Markdown
Author

Thank you for the review @j-kon
i think since it's a breaking change, it should land in 4.0 because most api breaks lands on 4.0

@thunderbiscuit thunderbiscuit added this to the 4.0.0 milestone Oct 9, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

Preserve detailed address parsing errors

3 participants